fix: add ellipsis tooltip component on mui tables#266
Conversation
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
📝 WalkthroughWalkthroughAdds an EllipsisTooltip component that measures overflow and conditionally shows a tooltip, integrates it into base, editable, and sortable MUI table variants, and tweaks action button styling/disabled handling; includes tests for the tooltip and table ellipsis behavior. ChangesEllipsis Tooltip Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant EllipsisTooltip
participant DOM as Element
participant MUITooltip
User->>EllipsisTooltip: hover / onMouseEnter
EllipsisTooltip->>DOM: read scrollWidth, offsetWidth
EllipsisTooltip->>EllipsisTooltip: compare widths
alt overflow detected
EllipsisTooltip->>MUITooltip: title = provided text
else no overflow
EllipsisTooltip->>MUITooltip: title = ""
end
MUITooltip->>User: render tooltip (or not) on hover
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/mui/editable-table/mui-table-editable.js`:
- Line 322: The EllipsisTooltip title currently uses String(row[col.columnKey]
?? "") which yields unhelpful results for objects/arrays; update the cell
rendering around EllipsisTooltip (where title is set) to use a safe formatter
(e.g., a small helper function like formatTooltipValue or reuse the base-table
formatter) that: checks for null/undefined, returns primitives via String(...),
and for objects/arrays attempts JSON.stringify with a try/catch (falling back to
Object.prototype.toString or an empty string) so the tooltip shows useful text
for complex values; update the title prop to call that formatter instead of
String(row[col.columnKey]).
- Around line 321-324: The tooltip currently always uses raw data
(String(row[col.columnKey])) while the cell shows col.render(row), causing a
mismatch; update the EllipsisTooltip title to mirror the rendered cell by using
col.render(row) when col.render is present (and falling back to
String(row[col.columnKey] ?? "") otherwise), converting any React node to text
the same way the base table fix does (e.g., via the existing helper or
ReactDOMServer.renderToStaticMarkup) so the tooltip content matches the
displayed cell; change the usage around EllipsisTooltip/title near col.render
and row[col.columnKey].
In `@src/components/mui/sortable-table/mui-table-sortable.js`:
- Line 223: The EllipsisTooltip title currently uses String(row[col.columnKey]
?? "") which yields unhelpful "[object Object]" for non-primitive values; update
the tooltip value handling in the component that renders EllipsisTooltip (look
for the JSX containing EllipsisTooltip and the variables row and col.columnKey)
to guard against objects by converting primitives with String(...) but for
objects/arrays attempt a safe JSON.stringify (with a try/catch and a fallback
like "(object)") so the title shows meaningful text for non-string values;
optionally extract this logic to a small helper (e.g., toSafeTitle(value)) and
use that when passing title to EllipsisTooltip.
- Around line 222-228: The tooltip currently uses the raw row[col.columnKey]
value instead of the rendered cell content; change the render so the
EllipsisTooltip title uses the same displayed value as the cell by computing a
displayedValue = col.render?.(row) || row[col.columnKey] (or similar) and then
pass title={String(displayedValue ?? "")} to EllipsisTooltip; update the code
paths that render the cell (the branch with EllipsisTooltip and the non-ellipsis
branch) to use this displayedValue so the tooltip matches the rendered output
(refer to col.render, row, columnKey, and EllipsisTooltip in
mui-table-sortable.js).
In `@src/components/mui/table/mui-table.js`:
- Line 172: The tooltip is using raw row[col.columnKey] instead of the displayed
rendered cell; update the EllipsisTooltip usage in mui-table.js to prefer a
column-provided tooltip string (e.g., col.tooltipContent?.(row) or
col.tooltipContent) and otherwise derive text from the rendered cell by calling
col.render(row) and converting the resulting React node to plain text
(strip/serialize children or use a small helper like renderToString/flattenText)
so the tooltip matches the visible cell; ensure you reference EllipsisTooltip,
col.render, col.tooltipContent and row when implementing the fallback.
- Line 172: The tooltip title currently uses String(row[col.columnKey] ?? "")
which yields "[object Object]" for objects/arrays; update the EllipsisTooltip
title logic (in the component rendering the table row/col — locate
EllipsisTooltip usage with row and col.columnKey) to first inspect the value: if
it's string/number/boolean use String(value), if it's object/array attempt
JSON.stringify(value) inside a try/catch and fall back to a short placeholder
like "" on error/too-large result; ensure the final value passed to
EllipsisTooltip.title is always a string.
- Around line 157-179: renderCell currently passes the raw value
row[col.columnKey] into EllipsisTooltip causing mismatched tooltip vs rendered
cell when col.render transforms data; update renderCell to derive the tooltip
text from the rendered content when col.render exists (or support an explicit
col.tooltipContent function) — e.g., compute tooltipTitle = col.tooltipContent ?
col.tooltipContent(row) : (col.render ?
extractTextFromRenderedOutput(col.render(row)) : String(row[col.columnKey] ??
"")) and pass tooltipTitle to EllipsisTooltip, keeping the visible cell content
as col.render(row) or content; touch renderCell, col.render, col.ellipsis and
EllipsisTooltip usage to implement this change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 873100c9-90af-4584-a713-72ca0a5e28c9
📒 Files selected for processing (6)
src/components/mui/__tests__/ellipsis-tooltip.test.jssrc/components/mui/__tests__/mui-table.test.jssrc/components/mui/editable-table/mui-table-editable.jssrc/components/mui/ellipsis-tooltip.jssrc/components/mui/sortable-table/mui-table-sortable.jssrc/components/mui/table/mui-table.js
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
ref: https://app.clickup.com/t/86bacrq3e
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
New Features
Tests